Skip to content

feat(container-runner): jitter idle timeout to avoid teardown waves - #5595

Open
abcxff wants to merge 1 commit into
stack/feat-container-runner-self-sleep-on-repeated-actor-start-qspsskoyfrom
stack/feat-container-runner-jitter-idle-timeout-to-avoid-teardown-waves-wxwlxowm
Open

feat(container-runner): jitter idle timeout to avoid teardown waves#5595
abcxff wants to merge 1 commit into
stack/feat-container-runner-self-sleep-on-repeated-actor-start-qspsskoyfrom
stack/feat-container-runner-jitter-idle-timeout-to-avoid-teardown-waves-wxwlxowm

Conversation

@abcxff

@abcxff abcxff commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@claude

claude Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review

Small, focused change: jitters the idle-sleep timer so co-started instances do not tear down in lockstep. Logic is sound (jitter only added, never subtracted; capped at min(20%, 60s); zero-base guarded). One thing worth fixing before merge, plus a couple of minor suggestions.

Blocking syscall I/O on the async runtime

random_duration_up_to (container-runner/src/main.rs) opens and reads /dev/urandom synchronously via std::fs::File/read_exact, and it is called directly from arm_idle_timeout, which runs inside the async start handler on the Tokio executor, not inside spawn_blocking. Every actor start now does a blocking open + read + close on the runtime thread.

The workspace already depends on rand (used throughout engine/*, rivetkit-core, etc.) via rand.workspace = true. rand::thread_rng() caches a CSPRNG-seeded generator per thread, so thread_rng().gen_range(0..=max_ms) avoids both the blocking I/O and the per-call open/close overhead of hand-rolling this against /dev/urandom. Since container-runner does not currently pull in rand, this would mean adding it to container-runner/Cargo.toml, but that is consistent with how every other crate in the repo sources randomness.

Practically the impact here is small (this is a one-shot timer per actor start, not a hot path, and /dev/urandom reads are fast), but it is an easy fix that also removes the hardcoded Linux-only path.

Missing test coverage

No test covers idle_timeout_with_jitter / random_duration_up_to bounds behavior (jitter never subtracts, capped at min(20%, 60s), zero-jitter fallback on RNG failure). Per CLAUDE.md, Rust tests belong under container-runner/tests/, not inline #[cfg(test)]. These are pure functions, cheap to test directly (e.g. assert idle_timeout_with_jitter(base) >= base across a range of base values, and that the cap holds for large base).

Minor

The silent fallback to Duration::ZERO when /dev/urandom cannot be opened/read is a reasonable fail-open choice, but it is silent, with no log line. If this ever happened fleet-wide (e.g. a sandboxed environment without /dev/urandom), jitter would silently stop working with no signal in the logs. Not blocking, just a visibility gap.

Overall a well-scoped fix for the stated problem (teardown waves); the main ask is to swap the hand-rolled /dev/urandom read for rand::thread_rng() so the RNG call is not a blocking syscall on the async path.

@abcxff
abcxff force-pushed the stack/feat-container-runner-jitter-idle-timeout-to-avoid-teardown-waves-wxwlxowm branch from c320933 to ddf4aad Compare August 25, 2026 19:36
@abcxff
abcxff force-pushed the stack/feat-container-runner-self-sleep-on-repeated-actor-start-qspsskoy branch from 892453d to 8617b0f Compare August 25, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant